-
Notifications
You must be signed in to change notification settings - Fork 121
Bookings: Multiple selection support #16285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
RafaelKayumov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Works as described.
Left a couple of questions.
Another question is - should all selected filters be persisted after the filter selector is closed and re-opened? I can see that the "Filter (N)" button displays the amount of filters configured. But when I open the selector, I can see that all filters are set to "Any" except customer name and date+time.
| selected.selectedValue = MultipleFilterSelection(items: selectedOptions) | ||
| self?.updateUI(numberOfActiveFilters: self?.viewModel.filterTypeViewModels.numberOfActiveFilters ?? 0) | ||
| self?.listSelector.reloadData() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this code is being repeated in a few places:
- case .multiSelectStaticOptions(let options):
- case .bookingResource(let siteID):
- case .bookableProduct(let siteID):
- case .bookingDateTime:
May be it's work putting in a separate private method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reuse an existing closure in 39f47d3.
| isSelected: selectedItems.isEmpty, | ||
| onSelection: { | ||
| selectedItems.removeAll() | ||
| onSelection(selectedItems) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it intended to pass cleaned selectedItems array when "Any" is selected? How about passing [] explicitly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch - updated in 49ead88.
Generated by 🚫 Danger |
Thanks for catching this! This is a bug with showing the selected filters with multiple selection - I fixed it in a0ed11e: Simulator.Screen.Recording.-.iPhone.17.-.2025-10-29.at.09.44.43.movThis PR is ready for another look 🙏 |
RafaelKayumov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx for the updates. LGTM.

Part of WOOMOB-1240
Description
This PR adds the support for multiple selection in Team member, Service/Event, Payment status, and Attendance status filters. Support for Customer name will be added in a separate PR.
Test Steps
Screenshots
Simulator.Screen.Recording.-.iPhone.17.-.2025-10-28.at.18.37.27.mov
RELEASE-NOTES.txtif necessary.